Implement Syntax 0.8, part 1 (items 1-5) #307
Conversation
Pike
left a comment
Pike
left a comment
There was a problem hiding this comment.
I glanced over this, but didn't find the time to do an actual review.
The comments changes here change again, I think?
Also, send an inquiry to @flodolo that we're OK to unsupport the text run changes without a compat implementation?
| // The Fluent Syntax spec uses /.*/ to parse comment lines. It matches all | ||
| // characters except the following ones, which are considered line endings by | ||
| // the regex engine. | ||
| const COMMENT_EOL = ["\n", "\r", "\u2028", "\u2029"]; |
There was a problem hiding this comment.
I just reviewed the opposite of this?
There was a problem hiding this comment.
This PR is only for part 1 of #303. The change you're talking about, projectfluent/fluent#219, will be implemented later on. I chose this approach to minimize the amount of work related to porting reference tests to fluent.js.
There was a problem hiding this comment.
In other words, const COMMENT_EOL = ["\n", "\r", "\u2028", "\u2029"]; corresponds to the Fluent Syntax spec as of projectfluent/fluent@bf46e18. I'll remove it before all of Syntax 0.8 is implemented.
| if (source[cursor] === "}") { | ||
| throw new FluentError("Unbalanced closing brace"); | ||
| } | ||
|
|
There was a problem hiding this comment.
nit only because it's hot. You can store source[cursor] and if/else here.
There was a problem hiding this comment.
Thanks. I think I prefer the current version, though.
There was a problem hiding this comment.
(There's no impact on the perf benchmark and the current version is quicker to read. I also wouldn't be surprised if JS engines optimized this anyways to some extend.)
3 participants
This implements changes listed as 1 through 5 from #303.